Skip to content

Add L-BFGS reconstruction engine #485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
jfowkes wants to merge 28 commits into
base: dev
Choose a base branch
from
Open

Add L-BFGS reconstruction engine #485

jfowkes wants to merge 28 commits into from

Conversation

@jfowkes
Copy link
Collaborator

@jfowkes jfowkes commented Mar 17, 2023

Resurrecting #400 as GitHub broke the original PR.

Add Limited-memory BFGS reconstruction engine to as an alternative to ML.

Caveats:

  • L-BFGS does not support the object smoothing preconditioner for technical reasons (this would require the ability to invert the Gaussian filter, i.e. exactly de-blur and this is known to be impossible). However probe preconditioning (i.e. probe-object scaling) is supported and this is enough to reconstruct on the moonflower example with a lower error than ML.

@jfowkes jfowkes marked this pull request as ready for review March 17, 2023 08:22
@jfowkes
Copy link
Collaborator Author

jfowkes commented Mar 17, 2023

@daurer this L-BFGS pull request is now ready for review and potential merge.

@daurer
Copy link
Contributor

daurer commented Feb 6, 2024

Needs a CuPy engine before it can be merged. @ptim0626 maybe something for you?

@ptim0626
Copy link
Contributor

ptim0626 commented Sep 11, 2024

A quick comment, the tolerance for LBFGS's tests is increased as it is very likely the discrepancy is a result of numerical instability instead of the problem of implementation.

@jfowkes jfowkes requested a review from daurer September 12, 2024 09:11
@jfowkes
Copy link
Collaborator Author

jfowkes commented Jan 29, 2025

@daurer this PR will soon need to be updated to use the wavefield preconditioner, I'm happy to do this.

jfowkes and others added 21 commits May 1, 2025 07:43
@jfowkes
Add L-BFGS reconstruction engine
4f9b017
@ptim0626 @jfowkes
WIP L-BFGS serial, passes some tests
0974ee0
@ptim0626 @jfowkes
Rework LBFGS serial using multiple inheritance
85871e7
@ptim0626 @jfowkes
Finish the GPU version of L-BFGS
0b6e297
@ptim0626 @jfowkes
Remove local path insert
b642394
@ptim0626 @jfowkes
Pass num of iteration to engine_iteratre
8f091a2
@ptim0626 @jfowkes
Fix a bug in LBFGS serial about updating
4e3fea4
@ptim0626 @jfowkes
Move smooth gradient to a function in LBFGS serial
341f9bc
@ptim0626 @jfowkes
Fix the LBFGS pycuda implementation
88ffca7
@jfowkes
Remove smooth gradient (accidentally left in)
e13687f
@jfowkes
Fix inefficient copy operations
e319e90
@jfowkes
Remove L-BFGS testing notebooks
544b62e
@jfowkes
Add L-BFGS example scripts and notebooks
cbabfb8
@ptim0626 @jfowkes
Add LBFGS cupy engine
8039129
@ptim0626 @jfowkes
Add LBFGS cupy example
859eaf0
@ptim0626 @jfowkes
Update LBFGS pycuda engine to use FFT Gaussian smoothing
7acfa93
@ptim0626 @jfowkes
Add missing import in LBFGS cupy
87d74b1
@ptim0626 @jfowkes
Update LBFGS tests
1f08d02
@ptim0626 @jfowkes
Add LBFGS pycuda tests
e0d4065
@ptim0626 @jfowkes
Add LBFGS cupy tests
bd0a99c
@ptim0626 @jfowkes
Increase LBFGS accelerated tests atol (am I cheating?)
0ddc382
@jfowkes
Copy link
Collaborator Author

jfowkes commented May 1, 2025

@daurer I've now added the wavefield preconditioner to LBFGS with tests and templates. Unfortunately pytest has stopped picking up the LBFGS tests in the CI, I assume this is the same issue that we had with the ML engine tests not being picked up?

@daurer
Copy link
Contributor

daurer commented Sep 18, 2025

@daurer I've now added the wavefield preconditioner to LBFGS with tests and templates. Unfortunately pytest has stopped picking up the LBFGS tests in the CI, I assume this is the same issue that we had with the ML engine tests not being picked up?

just saw this comment now. I think the tests should be name <something>_test.py

@jfowkes
Copy link
Collaborator Author

jfowkes commented Sep 18, 2025

just saw this comment now. I think the tests should be name <something>_test.py

Done. All tests now running (aside from the serial wavefield caveat as in #607) and passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daurer daurer Awaiting requested review from daurer

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Workshop 2024

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jfowkes @daurer @ptim0626